fix: reject unsupported configuration options by mohanagy · Pull Request #43 · mohanagy/miftah

mohanagy · 2026-07-10T20:54:15Z

Summary

Reject inert configuration controls with exact-path UNSUPPORTED_CONFIG_OPTION diagnostics.
Align validated public types, JSON Schema, generated presets, checked-in examples, documentation, and release notes with actual runtime behavior.
Preserve implemented controls plus hybrid routing, JSONL audit output, and force-on redaction declarations.

Validation

npm test (88 tests)
npm run typecheck
npm run lint
npm run build
node dist/cli/main.js schema
npm run check:pack
Built CLI validate and doctor rejection checks for process.restartOnCrash

Relates to #4

Summary by CodeRabbit

Configuration
- Unsupported configuration options are now rejected with UNSUPPORTED_CONFIG_OPTION, including the exact failing config path.
- Routing is hybrid-only; unsupported routing controls/plugins are rejected.
- Per-profile transport/command/url overrides are rejected; only supported override fields remain.
- Secret and audit redaction protections are always enforced (cannot be disabled); audit output is JSONL.
- Upstream/process restart behavior is simplified to be governed solely by process.startupTimeoutMs.
Documentation
- Updated README, docs, changelog, and example configs to match enforced validation and capability boundaries.
Tests
- Added schema contract and runtime parity tests, plus coverage for startup timeout defaulting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

coderabbitai · 2026-07-10T20:55:44Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ccb79040-e884-444e-8290-4484d149f2ca

📥 Commits

Reviewing files that changed from the base of the PR and between 0f586f8 and 2da4d6b.

📒 Files selected for processing (2)

src/config/schema.ts
tests/config-runtime-parity.test.ts

📝 Walkthrough

Walkthrough

Configuration types, schemas, validation, runtime wiring, presets, examples, documentation, and tests were updated to reject unsupported options, enforce hybrid routing and redaction, and retain only supported lifecycle and tooling controls.

Changes

Configuration Contract Enforcement

Layer / File(s)	Summary
Public configuration contracts and schema `src/config/types.ts`, `src/config/generate-json-schema.ts`	Configuration types and generated JSON Schema now expose supported profile overrides, hybrid routing, constrained process settings, and force-on redaction.
Unsupported-option validation `src/config/schema.ts`, `src/config/validate-config.ts`, `src/utils/errors.ts`	Validation rejects unsupported profile, routing, lifecycle, state, UI, tooling, and redaction options using `UNSUPPORTED_CONFIG_OPTION`, with post-parse contract checks.
Runtime defaults and process-manager wiring `src/cli/main.ts`, `src/config/presets.ts`, `src/upstream/upstream-process-manager.ts`, `examples/*.miftah.json`	Runtime managers, shared defaults, and examples retain only supported startup timeout and tooling settings.
Contract verification and documentation `tests/config-*.test.ts`, `tests/upstream-manager.test.ts`, `README.md`, `docs/config.md`, `CHANGELOG.md`	Tests verify schema, runtime parity, and timeout defaults, while documentation describes rejected options and supported controls.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConfigFile
  participant miftahConfigSchema
  participant validateConfig
  participant Runtime
  ConfigFile->>miftahConfigSchema: submit configuration
  miftahConfigSchema->>validateConfig: return parsed data or unsupported-option errors
  validateConfig->>Runtime: provide validated configuration

Possibly related issues

[Config] Reject or report configuration options that are not implemented #4 — The PR implements configuration validation, unsupported-option rejection, documentation, and tests.

Poem

A rabbit checks each config line,
Rejects the paths that do not align.
Hybrid hops and secrets stay bright,
JSONL audits guard the night.
Startup timeout leads the way.

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Docstring Coverage	❌ Error	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: rejecting unsupported configuration options.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/issue-4-config-runtime-parity

_{Comment @coderabbitai help to get the list of available commands.}

mohanagy · 2026-07-10T20:56:22Z

@coderabbitai review

coderabbitai · 2026-07-10T20:56:28Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-07-10T20:58:40Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/upstream/upstream-process-manager.ts (1)
40-43: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Default startupTimeoutMs is being overwritten

...options comes after the fallback, so when createRuntime passes { startupTimeoutMs: undefined }, the default 30_000 is lost and withTimeout(..., undefined) uses a ~1ms timer in Node. Move the spread before the fallback, or omit the key when it’s unset.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/upstream/upstream-process-manager.ts` around lines 40 - 43, In the
constructor’s `this.options` initialization, the trailing `...options`
overwrites the `startupTimeoutMs` fallback when the property is undefined.
Spread `options` before assigning `startupTimeoutMs: options.startupTimeoutMs ??
30_000`, preserving the default used by `withTimeout`.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Line 15: Update the breaking-change entry in CHANGELOG.md to document all
newly rejected profile fields, including profiles.default.metadata and
per-upstream transport, command, and url overrides, or link to the complete
configuration-boundaries documentation alongside the existing profile matcher
reference.

In `@docs/config.md`:
- Line 13: The documentation statement about upstream-specific `headers`
overrides incorrectly classifies all header values as non-secret; revise it to
limit non-secret classification to explicitly non-sensitive header values and
preserve secret-redaction guidance for authorization tokens and API keys.

In `@src/config/schema.ts`:
- Around line 114-120: Ensure unsupported process options are diagnosed before
typed validation: update the schema around rejectUnsupportedOption and
process.restartOnCrash to inspect raw values or defer their parsing with
z.unknown(), then emit UNSUPPORTED_CONFIG_OPTION via superRefine instead of
CONFIG_SCHEMA_INVALID; add a regression test covering the string value "true".

In `@tests/config-schema-contract.test.ts`:
- Around line 58-91: Extend the published config schema contract test to verify
that the generated security schema omits the runtime-rejected
requireProfileSwitchConfirmation field. Add the assertion alongside the existing
security checks in the test covering generateConfigSchema and the security
properties.

---

Outside diff comments:
In `@src/upstream/upstream-process-manager.ts`:
- Around line 40-43: In the constructor’s `this.options` initialization, the
trailing `...options` overwrites the `startupTimeoutMs` fallback when the
property is undefined. Spread `options` before assigning `startupTimeoutMs:
options.startupTimeoutMs ?? 30_000`, preserving the default used by
`withTimeout`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 835a9422-7be9-4148-a90c-dd9040ca8a7d

📥 Commits

Reviewing files that changed from the base of the PR and between 768e82d and a568b0e.

📒 Files selected for processing (15)

CHANGELOG.md
README.md
docs/config.md
examples/github.miftah.json
examples/multi-upstream.miftah.json
src/cli/main.ts
src/config/generate-json-schema.ts
src/config/presets.ts
src/config/schema.ts
src/config/types.ts
src/config/validate-config.ts
src/upstream/upstream-process-manager.ts
src/utils/errors.ts
tests/config-runtime-parity.test.ts
tests/config-schema-contract.test.ts

💤 Files with no reviewable changes (1)

examples/multi-upstream.miftah.json

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mohanagy · 2026-07-10T21:05:54Z

@coderabbitai review

coderabbitai · 2026-07-10T21:06:06Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/config/schema.ts (1)
147-169: 🎯 Functional Correctness | 🟠 Major

Apply unsupported-option diagnostics consistently to every rejected field.

Only process.* fields were widened to z.unknown(). Values such as profiles.*.metadata: "x", routing.plugins: {}, or security.requireProfileSwitchConfirmation: "true" still fail strict inner parsing before .superRefine() runs, producing CONFIG_SCHEMA_INVALID instead of the promised UNSUPPORTED_CONFIG_OPTION with the exact path. Use the same raw-value/pre-parse strategy for all rejected options and add malformed-value regression tests.

Also applies to: 171-224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config/schema.ts` around lines 147 - 169, Unsupported-option checks must
run before strict parsing for every rejected field, not only process.*. Update
the schema’s raw-value/pre-parse handling and related superRefine logic around
profile metadata, routing plugins/match, upstream overrides, and
security.requireProfileSwitchConfirmation so malformed values produce
UNSUPPORTED_CONFIG_OPTION with the exact path. Add regression tests covering
invalid values such as profiles.*.metadata, routing.plugins, and
security.requireProfileSwitchConfirmation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/config/schema.ts`:
- Around line 147-169: Unsupported-option checks must run before strict parsing
for every rejected field, not only process.*. Update the schema’s
raw-value/pre-parse handling and related superRefine logic around profile
metadata, routing plugins/match, upstream overrides, and
security.requireProfileSwitchConfirmation so malformed values produce
UNSUPPORTED_CONFIG_OPTION with the exact path. Add regression tests covering
invalid values such as profiles.*.metadata, routing.plugins, and
security.requireProfileSwitchConfirmation.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 00193d0a-0d83-49fd-88b3-c1152642d697

📥 Commits

Reviewing files that changed from the base of the PR and between a568b0e and 0f586f8.

📒 Files selected for processing (7)

CHANGELOG.md
docs/config.md
src/config/schema.ts
src/upstream/upstream-process-manager.ts
tests/config-runtime-parity.test.ts
tests/config-schema-contract.test.ts
tests/upstream-manager.test.ts

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mohanagy · 2026-07-10T21:14:03Z

@coderabbitai review

coderabbitai · 2026-07-10T21:14:11Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

fix: reject unsupported configuration options

a568b0e

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

coderabbitai Bot requested changes Jul 10, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread docs/config.md Outdated

Comment thread src/config/schema.ts

Comment thread tests/config-schema-contract.test.ts

fix: preserve config diagnostic guarantees

0f586f8

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

coderabbitai Bot reviewed Jul 10, 2026

View reviewed changes

fix: stabilize unsupported config diagnostics

2da4d6b

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mohanagy merged commit 39ed987 into development Jul 10, 2026
2 checks passed

mohanagy mentioned this pull request Jul 10, 2026

[Config] Reject or report configuration options that are not implemented #4

Closed

5 tasks

Conversation

mohanagy commented Jul 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Pre-merge checks failed

❌ Failed checks (1 error)

Uh oh!

mohanagy commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mohanagy commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mohanagy commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mohanagy commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading